kv: disallow follower reads for writing transactions #35969
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewed 4 of 4 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @nvanbenschoten)
pkg/storage/closed_timestamp_test.go, line 303 at r1 (raw file):
var notLeaseholderErrs int64 for i := range repls { repl := repls[i]
This is copied from code below but can we drop the func(r *storage)? r isn't used and there's no reason to both define and use repl and pass repls[i] to a function.
I guess I got burned by the captured iterator variable too many times?
pkg/storage/closed_timestamp_test.go, line 511 at r1 (raw file):
for i := range repls { repl := repls[i] func(r *storage.Replica) {
Can you drop the func here while you're around? It's even more egregious here because r gets shadowed by the retry
Fixes cockroachdb#35812. To avoid missing its own writes, a transaction must not evaluate a read on a follower who has nit caught up to at least its current provisional commit timestamp. We were violating this both at the DistSender level and at the Replica level. Because the ability to perform follower reads in a writing transaction is fairly unimportant and has these known issues, this commit disallows follower reads for writing transactions. Release note: None
nvanbenschoten
force-pushed
the
nvanbenschoten/noTxnFollowerRead
branch
from
733561f to
93a4113
Compare
There was a problem hiding this comment.
bors r+
Reviewable status:
pkg/storage/closed_timestamp_test.go, line 303 at r1 (raw file):
Previously, ajwerner wrote…
This is copied from code below but can we drop the
func(r *storage)?risn't used and there's no reason to both define and userepland passrepls[i]to a function.I guess I got burned by the captured iterator variable too many times?
Done.
pkg/storage/closed_timestamp_test.go, line 511 at r1 (raw file):
Previously, ajwerner wrote…
Can you drop the func here while you're around? It's even more egregious here because
rgets shadowed by the retry
Done.
35969: kv: disallow follower reads for writing transactions r=nvanbenschoten a=nvanbenschoten Fixes #35812. To avoid missing its own writes, a transaction must not evaluate a read on a follower who has nit caught up to at least its current provisional commit timestamp. We were violating this both at the DistSender level and at the Replica level. Because the ability to perform follower reads in a writing transaction is fairly unimportant and has these known issues, this commit disallows follower reads for writing transactions. Release note: None Co-authored-by: Nathan VanBenschoten <[email protected]>
Build succeeded |
Fixes #35812.
To avoid missing its own writes, a transaction must not evaluate a read
on a follower who has nit caught up to at least its current provisional
commit timestamp. We were violating this both at the DistSender level and
at the Replica level.
Because the ability to perform follower reads in a writing transaction is
fairly unimportant and has these known issues, this commit disallows
follower reads for writing transactions.
Release note: None